Skip to content

chore: Extended BaseMessage to support reasoning fields and updated usage accounting in streaming mode #3326

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 14 commits into from
Nov 20, 2025
Merged

chore: Extended BaseMessage to support reasoning fields and updated usage accounting in streaming mode #3326

merged 14 commits into from
Nov 20, 2025

Conversation

@fengju0213
Copy link
Collaborator

@fengju0213 fengju0213 commented Oct 24, 2025 •
edited
Loading

Description

Describe your changes in detail (optional if the linked issue already contains a detailed description of the changes).

  • Reasoning Support: Some providers now emit reasoning_content alongside
    assistant text (and even during streaming). We plumb that trace directly
    into BaseMessage so both batch and streaming callers can reliably access the
    thinking process without scraping metadata.
    • Streaming Usage Handling: Different APIs interleave usage chunks with choice
      deltas in inconsistent ways. We updated the stream processors to apply usage
      updates regardless of whether choices are present, ensuring final token
      accounting and completion emission work across providers like OpenAI, Gemini,
      and Qwen.

Checklist

Go over all the following points, and put an x in all the boxes that apply.

  • I have read the CONTRIBUTION guide (required)
  • I have linked this PR to an issue using the Development section on the right sidebar or by adding Fixes #issue-number in the PR description (required)
  • I have checked if any dependencies need to be added or updated in pyproject.toml and uv lock
  • I have updated the tests accordingly (required for a bug fix or a new feature)
  • I have updated the documentation if needed:
  • I have added examples if this is a new feature

If you are unsure about any of these, don't hesitate to ask. We are here to help!

@github-actions github-actions bot added the Review Required PR need to be reviewed label Oct 24, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 24, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch st_fix_deepseek_stream

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Wendong-Fan
Wendong-Fan reviewed Oct 25, 2025
View reviewed changes
Copy link
Member

@Wendong-Fan Wendong-Fan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this PR didn't fixed the issue calling deepseek reasoner model with stream=True and GET_REASONING_CONTENT=True

@Wendong-Fan Wendong-Fan added Waiting for Update PR has been reviewed, need to be updated based on review comment and removed Review Required PR need to be reviewed labels Oct 25, 2025
@fengju0213
Copy link
Collaborator Author

fengju0213 commented Oct 25, 2025

updated cc @Wendong-Fan

Wendong-Fan
Wendong-Fan requested changes Oct 27, 2025
View reviewed changes
Copy link
Member

@Wendong-Fan Wendong-Fan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove model specific logic added to ChatAgent

@waleedalzarooni waleedalzarooni self-requested a review November 4, 2025 04:50
@fengju0213 fengju0213 marked this pull request as draft November 4, 2025 06:42
@fengju0213
Copy link
Collaborator Author

fengju0213 commented Nov 12, 2025

@Wendong-Fan hi，based on our previous disscussion ，we should hanlde thinking content in model level. however ,after further further consideration, i realized that if we want to keep the original response structure，for example， the api returns results that separate thinking and conent，then we should this structure at model level. Once we retain this strcuture at model level, we inevitably need a set of processing logic in the chatagent. therefor , i will continue in the existing manner.

@fengju0213 fengju0213 mentioned this pull request Nov 13, 2025
Closed
3 tasks
@fengju0213 fengju0213 added Review Required PR need to be reviewed and removed Waiting for Update PR has been reviewed, need to be updated based on review comment labels Nov 13, 2025
@fengju0213 fengju0213 changed the title chore: fix deepseek_model.py stream mode chore: Extended BaseMessage to support reasoning fields and updated usage accounting in streaming mode Nov 13, 2025
@fengju0213 fengju0213 marked this pull request as ready for review November 13, 2025 16:34
@fengju0213 fengju0213 mentioned this pull request Nov 14, 2025
Closed
2 tasks
@fengju0213 fengju0213 requested a review from Saedbhati November 14, 2025 04:04
Saedbhati
Saedbhati approved these changes Nov 14, 2025
View reviewed changes
Copy link
Collaborator

@Saedbhati Saedbhati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @fengju0213! Could you provide me a model that supports the reasoning_content field for testing purposes?”

@fengju0213 fengju0213 self-assigned this Nov 17, 2025
@fengju0213 fengju0213 added this to the Sprint 42 milestone Nov 17, 2025
@fengju0213 fengju0213 linked an issue Nov 17, 2025 that may be closed by this pull request
[Feature Request] During streaming, return usage #3412
Closed
2 tasks
@fengju0213
Copy link
Collaborator Author

fengju0213 commented Nov 17, 2025 •
edited
Loading

LGTM @fengju0213! Could you provide me a model that supports the reasoning_content field for testing purposes?”

@Saedbhati thanks for review!you can use deepseek-reasoner to test it, if you need apikey, you can send me a message in slack

@waleedalzarooni
Copy link
Collaborator

waleedalzarooni commented Nov 18, 2025

Hey @Saedbhati, have you run the example files shall I go ahead and merge

@fengju0213
Copy link
Collaborator Author

fengju0213 commented Nov 18, 2025

Hey @Saedbhati, have you run the example files shall I go ahead and merge

hi @waleedalzarooni the key I gave you should be sufficient to run this example, right? Feel free to message me if you have any questions.

@waleedalzarooni waleedalzarooni enabled auto-merge (squash) November 18, 2025 13:18
@Saedbhati
Copy link
Collaborator

Saedbhati commented Nov 18, 2025

@waleedalzarooni Go head and merge LGTM!

@Wendong-Fan Wendong-Fan merged commit 030ba23 into master Nov 20, 2025
12 of 13 checks passed
@Wendong-Fan Wendong-Fan deleted the st_fix_deepseek_stream branch November 20, 2025 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Saedbhati Saedbhati Saedbhati approved these changes

@waleedalzarooni waleedalzarooni waleedalzarooni approved these changes

@Wendong-Fan Wendong-Fan Wendong-Fan approved these changes

Assignees

@fengju0213 fengju0213

Labels

Review Required PR need to be reviewed

Projects

None yet

Milestone

Sprint 42

Development

Successfully merging this pull request may close these issues.

[Feature Request] During streaming, return usage

5 participants

@fengju0213 @waleedalzarooni @Saedbhati @Wendong-Fan